Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add symbol name to the x/uibc QueryInflowsResponse #2377

Merged
merged 3 commits into from
Jan 3, 2024

Conversation

robert-zaremba
Copy link
Member

@robert-zaremba robert-zaremba commented Jan 3, 2024

Description

Following the outlfows update, here we update inflows

Summary by CodeRabbit

  • Refactor

    • Streamlined the data representation in the UI by adjusting the types of inflow and outflow information.
  • Bug Fixes

    • Improved the accuracy of financial data displayed by refining the underlying query methods.

@robert-zaremba robert-zaremba requested a review from a team as a code owner January 3, 2024 10:26
Copy link
Contributor

coderabbitai bot commented Jan 3, 2024

Walkthrough

The changes involve refining the representation and retrieval of financial data in the query.proto and grpc_query.go files. Notably, the handling of inflow and outflow data has been adjusted, including the removal of an import and the introduction of a new method for representing coins with token symbols. The QueryInflowsResponse message now uses a different format for its inflows data, and the retrieval methods for inflows and outflows have been refined.

Changes

File Summary
proto/umee/uibc/v1/query.proto Removed import of coin.proto; changed inflows field type to repeated DecCoinSymbol.
x/uibc/quota/grpc_query.go Updated AllOutflows to use coinsWithTokenSymbols; changed Inflows to use inflowCoins; initialized k with q.Keeper(&ctx).
x/uibc/quota/token.go Introduced token.go with a function mapping token denominations to their corresponding symbol names.

Poem

🐇 A rabbit hopped through the code, with glee, 🌟
Changing coins and types, as nimble as can be.
No more imports, a cleaner path we weave,
With symbols for tokens, in the data that we retrieve. 🎉

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • You can tag CodeRabbit on specific lines of code or entire files in the PR by tagging @coderabbitai in a comment. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • You can tag @coderabbitai in a PR comment and ask questions about the PR and the codebase. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid.
    • @coderabbitai read the files in the src/scheduler package and generate README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@robert-zaremba robert-zaremba added the skip-e2e-test Skip the e2e tests label Jan 3, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 007207a and c021f39.
Files ignored due to filter (2)
  • swagger/swagger.yaml
  • x/uibc/query.pb.go
Files selected for processing (2)
  • proto/umee/uibc/v1/query.proto (2 hunks)
  • x/uibc/quota/grpc_query.go (1 hunks)
Additional comments: 6
x/uibc/quota/grpc_query.go (4)
  • 59-59: Verify the implementation of coinsWithTokenSymbols method to ensure it correctly populates the outflows array with token symbols.
Verification successful

The coinsWithTokenSymbols method does exist in the codebase and is defined in the file x/uibc/quota/token.go with the receiver type Keeper. This confirms that the method is implemented and should be populating the outflows array with token symbols as intended in the AllOutflows method.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the definition of coinsWithTokenSymbols within the codebase.
ast-grep --lang go --pattern $'func ($_ Querier) coinsWithTokenSymbols($_, $_) $$$'

Length of output: 84



Script:

#!/bin/bash
# Broaden the search for the coinsWithTokenSymbols method without assuming the receiver type or the exact parameters.
ast-grep --lang go --pattern $'func $$$ coinsWithTokenSymbols($$$) $$$'

Length of output: 72



Script:

#!/bin/bash
# Use ripgrep to search for the coinsWithTokenSymbols method in the codebase.
rg 'func .*coinsWithTokenSymbols'

Length of output: 155

* 67-67: Ensure that the renaming of `inflows` to `inflowCoins` is consistent throughout the codebase and does not cause any issues.
Verification successful

The search results indicate that the term inflows is still present in various parts of the codebase. However, it is important to note that the context of the usage matters. The term inflows appears in comments, method names, and variable names in different files. The renaming in the Inflows method to inflowCoins seems to be specific to the grpc_query.go file and does not necessarily imply that all instances of inflows throughout the codebase would be renamed.

The presence of inflows in the query.pb.go, quota_test.go, quota.go, grpc_query.go, genesis.go, query.go, quota.pb.go, query.pb.gw.go, and genesis.pb.go files suggests that inflows is a term used in various contexts, such as protobuf definitions, test cases, keeper methods, and genesis state.

The specific change mentioned in the review comment refers to the Inflows method in the grpc_query.go file, where the variable was renamed to inflowCoins. The search results show that within the grpc_query.go file, the variable inflows has been updated to inflowCoins on line 80, which is consistent with the change described in the review comment.

Given the information provided, it appears that the renaming of inflows to inflowCoins within the Inflows method is consistent and does not cause issues within the scope of the changes made in the grpc_query.go file. However, the term inflows is still used in other contexts within the codebase, which is expected and does not indicate an issue.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the usage of the old inflows variable to ensure it has been updated everywhere.
rg --type go 'inflows' --vimgrep

Length of output: 3284

* 69-69: Confirm that the new method of initializing the keeper with `q.Keeper(&ctx)` is correctly implemented and consistent throughout the codebase.
Verification successful

The search results show that the q.Keeper(&ctx) initialization is used consistently across various methods in the grpc_query.go file within the x/uibc/quota directory, as well as in the grpc_query.go file within the x/metoken/keeper directory. This indicates that the new method of initializing the keeper with q.Keeper(&ctx) is indeed implemented consistently throughout the codebase.

Based on the provided output, the change to initialize the keeper with q.Keeper(&ctx) is:

  • : The verification is successful, and the new method of initializing the keeper is used consistently across the codebase.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the usage of q.Keeper to ensure the new initialization is used consistently.
rg --type go 'q.Keeper\(&ctx\)' --vimgrep

Length of output: 900

* 80-80: Verify the correct usage of `coinsWithTokenSymbols` method in the `Inflows` method to ensure inflows are populated with token symbols accurately.
Verification successful

The ast-grep command has successfully matched the Inflows method in the grpc_query.go file and provided the relevant code snippet. From the output, we can see the usage of the coinsWithTokenSymbols method:

inflows := k.coinsWithTokenSymbols(ctx, inflowCoins)

This line of code is within the Inflows method and shows that inflowCoins is passed to the coinsWithTokenSymbols method of the k (keeper) object, with ctx as the context. The result is assigned to the inflows variable, which is then used to construct the QueryInflowsResponse object returned by the method.

Based on the provided output, the coinsWithTokenSymbols method is indeed used within the Inflows method to populate the inflows with token symbols, which aligns with the review comment.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the usage of coinsWithTokenSymbols within the Inflows method.
ast-grep --lang go --pattern $'func ($_ Querier) Inflows($_, $_) $$$ { $$$ coinsWithTokenSymbols($_, $_) $$$ }'

Length of output: 112



Script:

#!/bin/bash
# Search for the Inflows method to inspect its implementation and verify the usage of coinsWithTokenSymbols.
ast-grep --lang go --pattern $'func ($_ Querier) Inflows($_, $_) $$$ { $$$ }'

Length of output: 1251

proto/umee/uibc/v1/query.proto (2)
  • 1-6: Removal of the import statement for "cosmos/base/v1beta1/coin.proto" is appropriate if it's no longer used after the changes.

  • 70-70: The change to the inflows field type in the QueryInflowsResponse message to repeated DecCoinSymbol aligns with the PR objectives. Ensure this new type is consistently used and properly handled throughout the codebase.

Verification successful

The search results confirm that the cosmos.base.v1beta1.DecCoin type has been replaced by DecCoinSymbol in the uibc module's query.proto file as intended by PR 2377. The type is still present in other .proto files, but this is outside the scope of the PR in question.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the usage of the old DecCoin type to ensure it has been replaced by DecCoinSymbol.
rg --type proto 'cosmos.base.v1beta1.DecCoin' --vimgrep

Length of output: 85



Script:

#!/bin/bash
# Search for the usage of the old DecCoin type to ensure it has been replaced by DecCoinSymbol.
# Removing the --type flag to include all files in the search.
rg 'cosmos.base.v1beta1.DecCoin' --vimgrep

Length of output: 1365

Copy link
Collaborator

@gsk967 gsk967 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@robert-zaremba robert-zaremba added this pull request to the merge queue Jan 3, 2024
Merged via the queue into main with commit cbf1d36 Jan 3, 2024
24 of 38 checks passed
@robert-zaremba robert-zaremba deleted the robert/inflows-symbol branch January 3, 2024 10:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between c021f39 and 561c900.
Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • x/uibc/quota/token.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • CHANGELOG.md
Additional comments: 1
x/uibc/quota/token.go (1)
  • 11-12: The function assumes that GetAllRegisteredTokens will always provide a complete mapping of base denoms to symbol denoms. Verify that GetAllRegisteredTokens has proper error handling and that there are no scenarios where a token could be registered without a symbol.
Verification successful

The GetAllRegisteredTokens method is designed to either return a non-nil list of tokens or panic if it encounters an error, as indicated by its implementation using store.MustLoadAll. Therefore, the assumption in the token.go file that the method will always provide a complete mapping is consistent with the method's behavior. There is no need for additional error handling in the calling function since the method does not return an error.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that GetAllRegisteredTokens handles errors appropriately and that all registered tokens have symbols.
rg --type go 'GetAllRegisteredTokens' --context 5

Length of output: 17739



Script:

#!/bin/bash
# Look at the implementation of GetAllRegisteredTokens in iter.go and any other potential implementations.
rg --type go 'func \(.*Keeper\) GetAllRegisteredTokens' --context 5

Length of output: 1355

x/uibc/quota/token.go Show resolved Hide resolved
x/uibc/quota/token.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-e2e-test Skip the e2e tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants